Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adapt to new scheduling API and latest changes in Unikraft #11

Closed

Conversation

eduardvintila
Copy link
Member

@eduardvintila eduardvintila commented Nov 23, 2022

This PR adapts pthread-embedded to the latest changes introduced in Unikraft, especially the ones from this PR: unikraft/unikraft#564

Signed-off-by: Eduard Vintilă eduard.vintila47@gmail.com

Signed-off-by: Eduard Vintilă <eduard.vintila47@gmail.com>
@eduardvintila eduardvintila changed the title Adapt to new scheduling API Adapt to new scheduling API and latest changes in Unikraft Apr 9, 2023
@eduardvintila eduardvintila marked this pull request as ready for review April 9, 2023 18:52
@John-Ted
Copy link

When trying to compile app-sqlite, I get the following errors:

/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/apps/app-sqlite/build/libsqlite/origin/sqlite-amalgamation-3400100/sqlite3.c: In function ‘pthreadMutexAlloc’:
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/apps/app-sqlite/build/libsqlite/origin/sqlite-amalgamation-3400100/sqlite3.c:28103:41: error: ‘PTHREAD_MUTEX_INITIALIZER’ undeclared (first use in this function); did you mean ‘SQLITE3_MUTEX_INITIALIZER’?
28103 | #define SQLITE3_MUTEX_INITIALIZER(id) { PTHREAD_MUTEX_INITIALIZER }
      |                                         ^~~~~~~~~~~~~~~~~~~~~~~~~
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/apps/app-sqlite/build/libsqlite/origin/sqlite-amalgamation-3400100/sqlite3.c:28200:5: note: in expansion of macro ‘SQLITE3_MUTEX_INITIALIZER’
28200 |     SQLITE3_MUTEX_INITIALIZER(2),
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/apps/app-sqlite/build/libsqlite/origin/sqlite-amalgamation-3400100/sqlite3.c:28103:41: note: each undeclared identifier is reported only once for each function it appears in
28103 | #define SQLITE3_MUTEX_INITIALIZER(id) { PTHREAD_MUTEX_INITIALIZER }
      |                                         ^~~~~~~~~~~~~~~~~~~~~~~~~
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/apps/app-sqlite/build/libsqlite/origin/sqlite-amalgamation-3400100/sqlite3.c:28200:5: note: in expansion of macro ‘SQLITE3_MUTEX_INITIALIZER’
28200 |     SQLITE3_MUTEX_INITIALIZER(2),
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/apps/app-sqlite/build/libsqlite/origin/sqlite-amalgamation-3400100/sqlite3.c:28225:9: warning: implicit declaration of function ‘pthread_mutexattr_init’; did you mean ‘pthreadMutexInit’? [-Wimplicit-function-declaration]
28225 |         pthread_mutexattr_init(&recursiveAttr);
      |         ^~~~~~~~~~~~~~~~~~~~~~
      |         pthreadMutexInit
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/apps/app-sqlite/build/libsqlite/origin/sqlite-amalgamation-3400100/sqlite3.c:28226:9: warning: implicit declaration of function ‘pthread_mutexattr_settype’ [-Wimplicit-function-declaration]
28226 |         pthread_mutexattr_settype(&recursiveAttr, PTHREAD_MUTEX_RECURSIVE);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/apps/app-sqlite/build/libsqlite/origin/sqlite-amalgamation-3400100/sqlite3.c:28226:51: error: ‘PTHREAD_MUTEX_RECURSIVE’ undeclared (first use in this function); did you mean ‘SQLITE_MUTEX_RECURSIVE’?
28226 |         pthread_mutexattr_settype(&recursiveAttr, PTHREAD_MUTEX_RECURSIVE);
      |                                                   ^~~~~~~~~~~~~~~~~~~~~~~
      |                                                   SQLITE_MUTEX_RECURSIVE
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/apps/app-sqlite/build/libsqlite/origin/sqlite-amalgamation-3400100/sqlite3.c:28227:9: warning: implicit declaration of function ‘pthread_mutex_init’; did you mean ‘pthreadMutexInit’? [-Wimplicit-function-declaration]
28227 |         pthread_mutex_init(&p->mutex, &recursiveAttr);
      |         ^~~~~~~~~~~~~~~~~~
      |         pthreadMutexInit
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/apps/app-sqlite/build/libsqlite/origin/sqlite-amalgamation-3400100/sqlite3.c:28228:9: warning: implicit declaration of function ‘pthread_mutexattr_destroy’ [-Wimplicit-function-declaration]
28228 |         pthread_mutexattr_destroy(&recursiveAttr);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/apps/app-sqlite/build/libsqlite/origin/sqlite-amalgamation-3400100/sqlite3.c: In function ‘pthreadMutexFree’:
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/apps/app-sqlite/build/libsqlite/origin/sqlite-amalgamation-3400100/sqlite3.c:28275:5: warning: implicit declaration of function ‘pthread_mutex_destroy’ [-Wimplicit-function-declaration]
28275 |     pthread_mutex_destroy(&p->mutex);
      |     ^~~~~~~~~~~~~~~~~~~~~
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/apps/app-sqlite/build/libsqlite/origin/sqlite-amalgamation-3400100/sqlite3.c: In function ‘pthreadMutexEnter’:
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/apps/app-sqlite/build/libsqlite/origin/sqlite-amalgamation-3400100/sqlite3.c:28324:3: warning: implicit declaration of function ‘pthread_mutex_lock’; did you mean ‘pthreadMutexAlloc’? [-Wimplicit-function-declaration]
28324 |   pthread_mutex_lock(&p->mutex);
      |   ^~~~~~~~~~~~~~~~~~
      |   pthreadMutexAlloc
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/apps/app-sqlite/build/libsqlite/origin/sqlite-amalgamation-3400100/sqlite3.c: In function ‘pthreadMutexTry’:
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/apps/app-sqlite/build/libsqlite/origin/sqlite-amalgamation-3400100/sqlite3.c:28370:7: warning: implicit declaration of function ‘pthread_mutex_trylock’; did you mean ‘pthreadMutexAlloc’? [-Wimplicit-function-declaration]
28370 |   if( pthread_mutex_trylock(&p->mutex)==0 ){
      |       ^~~~~~~~~~~~~~~~~~~~~
      |       pthreadMutexAlloc
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/apps/app-sqlite/build/libsqlite/origin/sqlite-amalgamation-3400100/sqlite3.c: In function ‘pthreadMutexLeave’:
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/apps/app-sqlite/build/libsqlite/origin/sqlite-amalgamation-3400100/sqlite3.c:28408:3: warning: implicit declaration of function ‘pthread_mutex_unlock’; did you mean ‘pthreadMutexAlloc’? [-Wimplicit-function-declaration]
28408 |   pthread_mutex_unlock(&p->mutex);
      |   ^~~~~~~~~~~~~~~~~~~~
      |   pthreadMutexAlloc
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/apps/app-sqlite/build/libsqlite/origin/sqlite-amalgamation-3400100/sqlite3.c: In function ‘sqlite3ThreadCreate’:
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/apps/app-sqlite/build/libsqlite/origin/sqlite-amalgamation-3400100/sqlite3.c:32756:10: warning: implicit declaration of function ‘pthread_create’; did you mean ‘timer_create’? [-Wimplicit-function-declaration]
32756 |     rc = pthread_create(&p->tid, 0, xTask, pIn);
      |          ^~~~~~~~~~~~~~
      |          timer_create
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/apps/app-sqlite/build/libsqlite/origin/sqlite-amalgamation-3400100/sqlite3.c: In function ‘sqlite3ThreadJoin’:
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/apps/app-sqlite/build/libsqlite/origin/sqlite-amalgamation-3400100/sqlite3.c:32776:10: warning: implicit declaration of function ‘pthread_join’; did you mean ‘pthread_atfork’? [-Wimplicit-function-declaration]
32776 |     rc = pthread_join(p->tid, ppOut) ? SQLITE_ERROR : SQLITE_OK;
      |          ^~~~~~~~~~~~
      |         
 pthread_atfork

I am using this PR and unikraft/lib-newlib#28. As this issue appears related to pthread, I'm writing this here.
Config file. Is there another option I need to enable?

@eduardvintila
Copy link
Member Author

eduardvintila commented Apr 27, 2023

Hey, @John-Ted, thanks for testing. Hmm, I've tried compiling with your config file and it works on my side. Can you check the order of the libraries in the Makefile? The order is important and lib-pthread-embedded should come before lib-newlib and lib-sqlite, like so:

LIBS := $(UK_LIBS)/lib-pthread-embedded:$(UK_LIBS)/lib-newlib:$(UK_LIBS)/lib-sqlite

@John-Ted
Copy link

Thank you, that was the issue.
Now I get the following pthread warnings:

/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/libs/lib-pthread-embedded/pte_osal.c: In function ‘pte_osThreadCreate’:
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/libs/lib-pthread-embedded/pte_osal.c:190:23: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
  190 |         capsule.magic = PTE_CAPSULE_MAGIC;
      |                       ^
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/libs/lib-pthread-embedded/pte_osal.c:199:44: warning: passing argument 2 of ‘uk_sched_thread_create_fn1’ makes ‘__attribute__((noreturn))’ qualified function pointer from unqualified [-Wdiscarded-qualifiers]
  199 |         th = uk_sched_thread_create_fn1(s, uk_stub_thread_entry, argv,
      |                                            ^~~~~~~~~~~~~~~~~~~~
In file included from /home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/libs/lib-pthread-embedded/pte_osal.c:35:
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/unikraft/lib/uksched/include/uk/sched.h:189:62: note: expected ‘uk_thread_fn1_t’ {aka ‘__attribute__((noreturn)) void (*)(void *)’} but argument is of type ‘void (*)(void *)’
  189 |                                              uk_thread_fn1_t fn1,
      |                                              ~~~~~~~~~~~~~~~~^~~
  CC      libpthread-embedded: pthread_sigmask.glue.o
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/libs/lib-pthread-embedded/attributes.c: In function ‘pthread_getattr_np’:
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/libs/lib-pthread-embedded/attributes.c:96:20: warning: assignment to ‘struct uk_thread *’ from incompatible pointer type ‘pte_osThreadHandle’ {aka ‘struct pte_thread_data *’} [-Wincompatible-pointer-types]
   96 |         _uk_thread = tp->threadId;
      |                    ^
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/libs/lib-pthread-embedded/attributes.c: In function ‘pthread_setname_np’:
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/libs/lib-pthread-embedded/attributes.c:124:20: warning: assignment to ‘struct uk_thread *’ from incompatible pointer type ‘pte_osThreadHandle’ {aka ‘struct pte_thread_data *’} [-Wincompatible-pointer-types]
  124 |         _uk_thread = tp->threadId;
      |                    ^
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/libs/lib-pthread-embedded/attributes.c: In function ‘pthread_getname_np’:
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/libs/lib-pthread-embedded/attributes.c:144:20: warning: assignment to ‘struct uk_thread *’ from incompatible pointer type ‘pte_osThreadHandle’ {aka ‘struct pte_thread_data *’} [-Wincompatible-pointer-types]
  144 |         _uk_thread = tp->threadId;
      |                    ^

Are these warnings relevant? SQLite works fine, and I did not find any other issues.

Copy link

@John-Ted John-Ted left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning comment addressed here: unikraft/lib-newlib#28 (comment). All good.

Reviewed-by: Ioan-Teodor Teugea ioan_teodor.teugea@stud.acs.upb.ro

Copy link

@mariasfiraiala mariasfiraiala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's merge this @eduardvintila. Please keep in mind the two issues with app-python3 and app-helloworld-cpp and try to fix them later on. These apps aren't critical, but it would be nice to have them working, nevertheless.

Reviewed-by: Maria Sfiraiala maria.sfiraiala@gmail.com

@teotiron
Copy link

teotiron commented May 6, 2023

Tested this with app-sqlite, app-nginx and app-redis and it properly works. Let's merge this.

Copy link

@teotiron teotiron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed-by: Teodor Tiron teotiron@gmail.com

@razvand razvand added the enhancement New feature or request label May 7, 2023
Copy link

@razvand razvand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved-by: Razvan Deaconescu razvand@unikraft.io

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done!
Development

Successfully merging this pull request may close these issues.

5 participants